Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add otelcol.receiver.solace #2084

Merged
merged 11 commits into from
Nov 15, 2024
Merged

Add otelcol.receiver.solace #2084

merged 11 commits into from
Nov 15, 2024

Conversation

wildum
Copy link
Contributor

@wildum wildum commented Nov 13, 2024 •
edited
Loading

PR Description

Add the solace receiver from the Otel contrib repo: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/solacereceiver

Which issue(s) this PR fixes

Fixes #1993

Notes to the Reviewer

A few things to note:

  • I tested it manually using the Solace image. This was not super easy to setup, I wrote a guide in the component for it.
  • The upstream component uses a list for the broker but only the first element in the list is used. On Alloy side I decided to use a string to avoid confusion
  • The upstream component has a retry policy when the next component fails to consume it. I believe that this can be a problem in Alloy when the component has several consumers: if one of the consumer fails, I think that they will all receive the data again, which might lead to data duplication. I added a warning about this in the doc. I don't think that there is a way around it but I also don't think that this is a big problem because it's unlikely to get two traces consumer at this level. If there is a need for branching, an easy workaround would be to just do the branching in the next component.
  • I needed to fix a test because of the special case of having empty ptr structs

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@wildum wildum requested review from clayton-cornell and a team as code owners November 13, 2024 16:13
@thampiotr
Copy link
Contributor

The upstream component has a retry policy when the next component fails to consume it.

That's odd, no? Any idea why did they add this? Shouldn't this be something done on the runtime level?

@wildum
Copy link
Contributor Author

wildum commented Nov 15, 2024

That's odd, no? Any idea why did they add this? Shouldn't this be something done on the runtime level?

Yet not a fan of this + it makes their implementation much more complex. This was done 2 years ago, maybe they wanted some extra safety because the component might be spamming a lot? They did not add any other flow control strategy since then but there is also no way around it.

@wildum wildum merged commit e9472bd into main Nov 15, 2024
18 checks passed
@wildum wildum deleted the add-otel-solace-receiver branch November 15, 2024 13:05
@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clayton-cornell clayton-cornell clayton-cornell left review comments

@dehaansa dehaansa dehaansa left review comments

@thampiotr thampiotr thampiotr approved these changes

Assignees
No one assigned
Labels
type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add otelcol.receiver.solace
4 participants
@wildum @thampiotr @dehaansa @clayton-cornell